Skip to content

oneshot Channel #143741

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

oneshot Channel #143741

wants to merge 6 commits into from
+812 −0

Conversation

connortsui20
Copy link
Contributor

@connortsui20 connortsui20 commented Jul 10, 2025 •
edited
Loading

Tracking Issue: #143674

This PR adds an experimental oneshot module.

Before talking about the API itself, I would prefer to get some of these questions below out of the way first. And as discussed in the ACP it would be

Unresolved Questions

  • Why exactly is it okay for Sender to be Sync? Or basically, how do we boil down the discussion in Implement Sync for mpsc::Sender #111087 into a comment for the unsafe impl<T: Send> Sync for Sender<T> {}?
  • Why is mpsc::Receiver !Sync but mpmc::Receiver is Sync? Should oneshot::Receiver be Sync or not?
  • Should this PR try to add an is_ready method as proposed in the tracking issue? If so, then the surface of this PR would likely need to increase to add a pub(crate) fn is_disconnected method to mpmc (might even be a good idea to add that to all 3 channel flavors).
  • In a similar vein to the previous question, should the first internal implementation simply be a wrapper around mpmc, or should it be a wrapper around the internal crossbeam implementation?
  • Should the Sender and Receiver operations be methods or associated methods? So sender.send(msg) or Sender::send(sender, msg)? The method syntax is more consistent with the rest of the ecosystem (namely tokio)

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 10, 2025
@connortsui20 connortsui20 marked this pull request as ready for review July 10, 2025 17:09
@rustbot
Copy link
Collaborator

rustbot commented Jul 10, 2025

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 10, 2025
@tgross35
Copy link
Contributor

Maybe want to ask the unresolved questions at the ACP thread? To make sure it's seen by those already involved.

(I'll take a closer look later)

@connortsui20 connortsui20 mentioned this pull request Jul 10, 2025
Open
7 tasks
@RustyYato
Copy link
Contributor

RustyYato commented Jul 10, 2025 •
edited
Loading

Why exactly is it okay for Sender to be Sync? Or basically, how do we boil down the discussion in #111087 into a comment for the unsafe impl<T: Send> Sync for Sender {}?

Why is mpsc::Receiver !Sync but mpmc::Receiver is Sync? Should oneshot::Receiver be Sync or not?

Because mpsc is multiple producer single consumer, and does the receiver operation via &self, so it would be incorrect to allow recv to be shared across threads because that would allow two threads to recv at the same time, which isn't allowed for a mpsc.

But mpmc is multiple producer multiple consumer, so it doesn't matter how many threads recv at the same time.

For this one, they can both be unconditionally Sync, since there is no API to go from &Sender<T> or &Reciever<T> to &T. (even via traits like Debug or Clone). In fact it looks like &Sender<T> and &Receiver<T> are entirely useless (only a Debug impl, which doesn't provide any information), which is just like Exclusive.. This is more evidence that both Sender<T> and Receiver<T> can be unconditionally Sync

@connortsui20
Copy link
Contributor Author

connortsui20 commented Jul 10, 2025 •
edited
Loading

Thank you for the answer!

Please let me know if I'm interpreting this right: If all methods where synchronization must occur consume self, we can say that both Sender and Receiver are Sync. For example, if threads share a &oneshot::Receiver, then none of them can call recv anyways. And this would still be compatible with the proposed is_ready method discussed in the tracking issue.

oxalica
oxalica reviewed Jul 10, 2025
View reviewed changes
@connortsui20
oneshot channel first commit
7ced1c7
@connortsui20
add tests for oneshot
54e26fc 
This commit adds tests for the very basic new oneshot module. Most of
the tests were taken/heavily inspired by tests from the `oneshot crate.
@connortsui20
remove TODOs
3a4cc54
@connortsui20
impl Send and Sync cleanup
bdeb3a5 
This commit removes the unnecessary `unsafe impl Send` for `Sender` and
`Receiver` as they both `Send` that from the inner `mpmc`.

Additionally, this adds an unconditional `impl Sync` for `Sender` and
`Receiver`.
@connortsui20
add compile_fail examples
7e3faa8
@connortsui20
add doc examples
1a515fd
@connortsui20 connortsui20 mentioned this pull request Jul 21, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oxalica oxalica oxalica left review comments

Assignees

@tgross35 tgross35

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@connortsui20 @rustbot @tgross35 @RustyYato @oxalica